feat(rewrite): show a method call on one line - #14817
Draft
RonnyPfannschmidt wants to merge 6 commits into
Draft
Conversation
RonnyPfannschmidt
force-pushed
the
ronny/rewrite-flat-method-call
branch
from
July 31, 2026 19:06
3a5cbb0 to
e7ae134
Compare
psf-chronographer
Bot
added
the
bot:chronographer:provided
7 tasks
RonnyPfannschmidt
force-pushed
the
ronny/rewrite-flat-method-call
branch
from
July 31, 2026 20:18
e7ae134 to
2480b28
Compare
This was referenced Jul 31, 2026
RonnyPfannschmidt
force-pushed
the
ronny/rewrite-flat-method-call
branch
from
August 10, 2026 07:24
2480b28 to
aacbd89
Compare
RonnyPfannschmidt
force-pushed
the
ronny/rewrite-flat-method-call
branch
from
August 20, 2026 04:35
aacbd89 to
77dd763
Compare
This was referenced Aug 20, 2026
RonnyPfannschmidt
force-pushed
the
ronny/rewrite-flat-method-call
branch
4 times, most recently
from
August 26, 2026 10:04
aa87473 to
3403e04
Compare
visit_operand() only froze a bare name, so two other unhoisted operands
kept being evaluated after everything that follows them:
assert collect((x := 1), identity(x := 2)) == (1, 2)
assert collect(*items, identity(items := [9])) == (1, [9])
A walrus operator left in place assigns once the enclosing expression is
assembled, which is after the later arguments have run -- so the earlier
argument saw the later assignment. A starred argument hid its value
inside an ast.Starred, where the existing Name check could not see it.
Closes the order-starred-argument group and the remaining
order-call-argument entry in the coverage matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eeds visit_operand freezes a walrus operand whenever anything follows it, and a comparison always has at least one comparator -- so by the time visit_Compare looks at its left operand, a NamedExpr has already been copied into a temporary. The special case that did it here can never run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rewriter only ever visits expressions inside an assert condition, so an attribute always arrives in Load context and the fallback never runs. Removing it keeps the next visitor from copying a guard that cannot fire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A subscript was opaque: the message showed the value it produced with no indication of which container or key it came from. Decompose it the way attribute access already is. The container goes through visit_operand() because taking the expression away from generic_visit() takes away the hoisting that kept it ordered -- without that, `assert box[identity(box := other)] == 1` would start reading the post-walrus container. The order-axis guard in the coverage matrix fails if this is dropped. Slices keep the generic treatment; decomposing start/stop/step is rarely what a failure message needs. Closes the introspect-subscript group in the coverage matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A conditional expression showed only its result, so a failure gave no hint which way it went. Introspect the condition and report it as "(... if <cond> else ...)". The branches keep their original nodes: only the selected one may run, so neither can be hoisted into a statement. That leaves them evaluated after the condition, which is Python's order, so unlike the subscript container they need no freeze -- the order-axis guard covers it. Closes the introspect-ifexp group in the coverage matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
obj.method() reported the bound method as an intermediate of its own:
where 42 = compute()
where compute = Obj().compute
which spends a line on something nobody asked about. Build the
explanation from the receiver and the attribute name instead:
where 42 = Obj().compute()
The bound method keeps its own temporary even though it no longer has
its own explanation, because Python looks it up before evaluating the
arguments -- inlining the attribute into the rewritten call would move
the lookup after them, and with it the read of the receiver. Both
order-axis guards in the coverage matrix fail if that temporary goes.
Closes the introspect-method-call-flat group in the coverage matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RonnyPfannschmidt
force-pushed
the
ronny/rewrite-flat-method-call
branch
from
September 1, 2026 11:54
3403e04 to
d1741e4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #14816 → #14815 → #14814 → #14447 → #14921 → #14813. Its diff includes theirs.
obj.method()reported the bound method as an intermediate of its own:which spends a line on something nobody asked about. Build the explanation from the receiver and the attribute name instead:
The bound method keeps its own temporary even though it no longer has its own explanation, because Python looks it up before evaluating the arguments. Inlining the attribute into the rewritten call — which is what the obvious implementation does — moves the lookup after the arguments, and with it the read of the receiver. That version fails two guards:
test_method_receiver_in_order, a guard already in test(rewrite): systematic coverage matrix for assertion rewriting #14813 —assert obj.take(identity(obj := None))raisesAttributeErroronNonetest_method_lookup_precedes_arguments, added here — a property-based bound method is looked up after the arguments runI wrote it the inline way first and the guards caught it, which is the whole argument for landing the matrix ahead of the visitors.
testing/python/raises_group.py::test_assert_matchesasserts on an exact rewritten message and is updated for the flat format.Lands the
introspect-method-call-flatcases of the coverage matrix in #14813 as passing tests, plus the new lookup-order guard above (+6).Supersedes part of #14448.